-
-
Notifications
You must be signed in to change notification settings - Fork 35.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Texture: Add Texture.DEFAULT_COLOR_SPACE #28272
Conversation
📦 Bundle sizeFull ESM build, minified and gzipped.
🌳 Bundle size after tree-shakingMinimal build including a renderer, camera, empty scene, and dependencies.
|
Interesting idea! Let's use the three.js/examples/jsm/loaders/GLTFLoader.js Lines 3374 to 3378 in ed211d8
... otherwise they'll assume the default is NoColorSpace, and fail to set up non-color textures (like
I'm leaning toward (A). |
Thank you for your response.
Done.
TBH, I don't know which loaders should be updated... I've seen examples with quite a bit of duplicate code like: I found three loaders(EXRLoader, KTX2Loader and USDZLoader) that reference |
Personally I would recommend that we keep the examples this way, perhaps unless they're intentionally using a wide gamut like Display P3, which is only experimentally supported right now. I worry that if we 'simplify' the examples by doing We'll need to check all the loaders in #23283, if they support textures, but it doesn't need to be in this PR. The goal would be to make sure the loaders always specify the |
Generally I'm very against these kinds of editable global flags since they make function behavior less predictable and less reliable. They just pass on the verbosity to dependent projects. Ie if I have a project and I need loaded or processed data to be set to const originalDefaultColorSpace = Texture.DEFAULT_COLOR_SPACE;
Texture.DEFAULT_COLOR_SPACE = NoColorSpace;
// load or generate the data
Texture.DEFAULT_COLOR_SPACE = originalDefaultColorSpace; It also sets projects to break more easily. For example if someone is using three-gpu-pathtracer or the mesh bvh shader textures and they've set the default texture color space to sRGB then suddenly all my path tracer data textures break because they're implicitly set to the wrong color space. I think some examples of what this is actually helping should be provided so we can determine if this is actually addressing an issue and discuss other options. Without knowing more it should be very simple for end users to create utility functions that set the appropriate color space. |
I understand the motivation behind this PR, but I think the implications of this change would be too confusing. |
To be honest, I didn't think much of it, and after reading some feedback, I found that adding |
Ok, I believe you are right – thanks @gkjohnson, @WestLangley, and @linbingquan! |
Related issue: #XXXX
Description
Add
Texture.DEFAULT_COLORSPACE
variable.In some cases, reduce the amount of duplicate code.